Skip to content

Fix gas estimation failures and hydration mismatch#153

Merged
realproject7 merged 2 commits intomainfrom
task/fix-gas-estimation-hydration
Mar 16, 2026
Merged

Fix gas estimation failures and hydration mismatch#153
realproject7 merged 2 commits intomainfrom
task/fix-gas-estimation-hydration

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Gas estimation: Set explicit gas limits on donate (150k), mint/burn (2M) to bypass MetaMask's broken estimation on Base Sepolia
  • Hydration: DeadlineCountdown renders --:--:-- on server, starts countdown after client mount

Test plan

  • npm run typecheck passes
  • Donate/Buy/Sell flows work without gas errors
  • /story/[id] loads without hydration error in console

🤖 Generated with Claude Code

- Set explicit gas limits on donate (150k), mint (2M), burn (2M) to bypass
  MetaMask's broken gas estimation on Base Sepolia
- Fix DeadlineCountdown hydration mismatch: render "--:--:--" on server,
  start countdown only after client mount

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b APPROVE

All three changes are correct:

  1. DeadlineCountdown hydration fix — Standard Next.js pattern: init state as null, compute in useEffect after mount, show --:--:-- placeholder during SSR. Correct fix for time-dependent hydration mismatch.

  2. Gas limits (donate 150k, mint/burn 2M) — Reasonable values for these operations. Hardcoded limits are a pragmatic workaround for MetaMask's broken gas estimation on Base Sepolia. Acceptable tradeoff for testnet.

No issues found.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The gas-limit additions in Donate/Trading look narrowly scoped and the hydration intent is reasonable, but the new DeadlineCountdown implementation introduces a lint-failing React hooks violation. CI is red on this PR for that reason.

Findings

  • [medium] The hydration fix triggers react-hooks/set-state-in-effect, so the PR does not pass CI and cannot merge as submitted.
    • File: src/components/DeadlineCountdown.tsx:11
    • Suggestion: Restructure the countdown so the initial placeholder render does not require a synchronous setRemaining(...) inside useEffect (for example, drive display from a mounted/tick state or another pattern that satisfies the hook rule).

Decision

Requesting changes because the PR currently fails the required lint-and-typecheck check on src/components/DeadlineCountdown.tsx:11. Once that is addressed, the remaining changes look appropriately scoped to the reported MetaMask/Base Sepolia issue.

Use lazy initializer for useState instead of synchronous setState
inside useEffect to avoid cascading renders and satisfy the
react-hooks/set-state-in-effect rule.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The follow-up commit resolves the blocking DeadlineCountdown hook violation without expanding the PR scope. The explicit gas-limit changes remain focused on the MetaMask/Base Sepolia estimation issue, and CI is now green.

Findings

  • [info] No remaining blocking findings after f4721b0.
    • File: src/components/DeadlineCountdown.tsx:8
    • Suggestion: None.

Decision

Approving because the prior react-hooks/set-state-in-effect failure has been removed and the required lint-and-typecheck check now passes for PR #153.

@realproject7 realproject7 merged commit 2eac7da into main Mar 16, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants